Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a radius argument to find_snap() #402

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add a radius argument to find_snap() #402

wants to merge 3 commits into from

Conversation

dfsnow
Copy link

@dfsnow dfsnow commented Oct 8, 2024

This PR adds a radius argument to r5r's find_snap() function. radius controls the maximum search distance when snapping points in a data.frame to the OSM road network.

The default search radius set in R5 is just 1600m, which can be frustratingly small when working with large matrices in a grid (or other shape whose centroids may not align with the road network).

Note that this argument is already present in r5py.

@rafapereirabr
Copy link
Member

Hi @dfsnow . Thanks for openin this PR! The find_snap() is a support function we have created to help users determine the locations where R5 snaps input points (origins and destinations). So I just wanted to clarify that this change you implemented to the find_snap() does not affect the behavior of the routing functions, such as travel_time_matrix(), accessibility(), etc.

@dfsnow
Copy link
Author

dfsnow commented Oct 16, 2024

Hi @dfsnow . Thanks for openin this PR! The find_snap() is a support function we have created to help users determine the locations where R5 snaps input points (origins and destinations). So I just wanted to clarify that this change you implemented to the find_snap() does not affect the behavior of the routing functions, such as travel_time_matrix(), accessibility(), etc.

It shouldn't! As far as I can tell find_snap() and the associated Java aren't used anywhere within r5r. Plus, radius is an optional argument with a default value set to the current value used in r5, so if radius isn't specified then there's no change in the output of find_snap().

@rafapereirabr
Copy link
Member

Ok, good! Just wanted to make sure we're on the same page! Thanks again for the PR. We'll be looking at it more carefully soon, after we deal with issues #386 and #401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dfsnow @rafapereirabr